Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/main.py`:
- Around line 549-551: The current code appends current_time to
llm_rate_limit[user_id] and increments llm_daily_limit[user_id] before the LLM
API call, so failed calls consume quota; move the increment of llm_daily_limit
and the final append (or make the append provisional) so they only occur after a
successful API response from the LLM, or wrap the API call in
try/except/finally: tentatively append current_time to llm_rate_limit[user_id],
call the API, on success perform llm_daily_limit[user_id] += 1 (and keep the
timestamp), on exception remove the tentative timestamp from
llm_rate_limit[user_id] (and re-raise) so failures do not decrement user quota;
reference symbols: llm_rate_limit, llm_daily_limit, current_time, user_id, and
the LLM API call block.
- Around line 59-68: The llm_daily_limit counter never resets so users can be
permanently blocked; update the logic in respond_with_llm_message to track daily
usage keyed by date instead of a raw int: change llm_daily_limit to map user_id
-> {"count": int, "date": "YYYY-MM-DD"} (or store a tuple) and on each request
compare the stored date to today's date, resetting count to 0 when the date
differs before enforcing LLM_RPD_LIMIT, then increment the count after a
successful allowance; reference llm_daily_limit, respond_with_llm_message, and
LLM_RPD_LIMIT when making this change and mirror the cleanup semantics used for
llm_rate_limit.

---

Nitpick comments:
In `@src/main.py`:
- Line 589: Remove the redundant assignment to user_id (the line assigning
update.effective_user.id) since user_id is already set earlier; delete the
second assignment and ensure subsequent code in the function uses the originally
declared user_id variable (refer to the earlier user_id declaration and any uses
of update.effective_user.id in this function to confirm no other references need
adjustment).
- Around line 625-626: Move the traceback import out of the exception handler:
add "import traceback" to the module-level imports at the top of src/main.py and
remove the inline "import traceback" from the except Exception as e: handler so
the exception block uses the top-level traceback import; locate the inline
import in the except Exception as e: block to delete it and ensure any traceback
calls there continue to work with the new module-level import.
- Around line 504-520: The provider validation currently only checks for "grok"
and "gemini" so unsupported LLM_PROVIDER values fall through; add an explicit
allowed-provider check (e.g., ALLOWED_PROVIDERS = {"grok","gemini"}) and
validate LLM_PROVIDER against it before the existing checks, returning an error
message via update.message.reply_text if the provider is not supported,
preserving the language conditional; update the block that references
LLM_PROVIDER, GROK_API_KEY, GEMINI_API_KEY and reply handling to short-circuit
on invalid provider and avoid silently routing to the Gemini branch.
- Around line 765-770: The bare except block currently catches everything
(including KeyboardInterrupt/SystemExit); change it to catch only exceptions by
replacing "except:" with "except Exception:" (and optionally log the exception)
in the handler that returns the localized apology message (the block referencing
language and returning the Ukrainian/English strings) so system-level interrupts
are not suppressed.
- Around line 650-664: call_grok_api currently assumes grok_client is
initialized and will raise AttributeError if grok_client is None; add a
defensive guard at the start of call_grok_api that checks if grok_client is None
and handles it (e.g., log an error using the existing logger/update, raise a
clear RuntimeError, or return a sensible fallback string) before attempting
grok_client.chat.completions.create; reference the grok_client variable and
GROK_MODEL in the check to make the intent clear.

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/main.py`:
- Around line 634-646: The helper functions call_grok_api and call_gemini_api
currently swallow exceptions and return error strings, causing
respond_with_llm_message to treat failures as successes; change those helpers to
propagate errors (remove the outer try/except or re-raise caught exceptions) so
callers can detect failures, and update respond_with_llm_message so it only
increments llm_daily_limit[user_id]["count"], records the timestamp, and appends
to conversation_context[user_id] after a successful API call (i.e., when no
exception was raised and a valid bot_response was obtained); alternatively, if
you keep error-returning helpers, change them to return a clear success/failure
sentinel (e.g., (success, response)) and branch in respond_with_llm_message to
only perform the increment/store on success.

---

Nitpick comments:
In `@src/main.py`:
- Line 609: Remove the redundant reassignment "user_id =
update.effective_user.id" (the variable user_id was already set earlier) and use
the originally assigned user_id variable; if the intent was to refresh the user
ID, replace the duplicate assignment with a comment or an explicit check that
update.effective_user differs before reassigning. Locate the duplicate by
searching for the symbol user_id and the expression update.effective_user.id in
src/main.py and delete or replace the second occurrence.
- Around line 62-70: The global dicts conversation_context, llm_rate_limit and
llm_daily_limit can grow unbounded; add a periodic cleanup job (e.g., a function
named cleanup_stale_users or start_cleanup_task) that runs on a timer and
removes entries for users that haven't had activity for a configurable TTL (use
a new per-user last_seen timestamp tracked when requests arrive), or replace
these structures with a bounded cache/LRU implementation (e.g., wrap them with
an LRU cache class or use cachetools TTLCache) to automatically evict old users;
ensure the cleanup references the existing symbols conversation_context,
llm_rate_limit, llm_daily_limit and updates/clears their entries safely (with
the same locking/async model the app uses) and expose TTL and interval via
environment variables.

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@README.md`:
- Around line 31-43: The README's fenced code blocks for the Docker commands
(the three docker run examples and the instagram cookies example) are missing
language identifiers and trigger MD040; update each triple-backtick fence that
wraps those commands to use a bash language tag (e.g., replace ``` with ```bash)
so all four code blocks are annotated (the docker run without volume, the docker
run using Docker Hub image, the docker run with persistent volume, and the
instagram cookies docker run).

In `@src/main.py`:
- Around line 550-551: When merging persisted last-seen data back into the
in-memory map, avoid clobbering a fresher value with an older DB value: instead
of unconditionally assigning the DB value (the variable referenced as
persisted_last_seen or db_last_seen) to user_last_seen[user_id], only set it if
the key is missing or the DB timestamp is newer than the existing
user_last_seen[user_id]; apply the same check everywhere you merge DB values
(the code around user_last_seen, user_id, current_time and the DB load/merge
sites noted in the review) so current_time updates are preserved and stale DB
timestamps are ignored.
- Around line 650-653: The prompt text is hardcoded to Ukrainian in the
construction of safe_prompt (and context_str usage), breaking localization when
LANGUAGE=en; modify the logic that builds safe_prompt to choose the instruction
language dynamically based on the current language/config (e.g., the existing
LANGUAGE or locale variable) instead of the fixed Ukrainian string — use a
conditional or mapping to insert either the Ukrainian instruction ("Відповідай
українською...") when language is 'uk' or the equivalent English instruction
("Answer in English...") when language is 'en' (apply the same change both in
the branch that includes context_str and the else branch so safe_prompt respects
the selected language consistently).
- Around line 553-557: The code is calling synchronous DB methods
(db_storage.load_user_data, save_user_data, get_stale_users, delete_user_data)
directly inside async handlers (e.g., in llm_daily_limit load path and in async
functions respond_with_llm_message and cleanup_stale_users) which blocks the
event loop; fix by offloading those synchronous calls to a thread executor
(e.g., using asyncio.get_running_loop().run_in_executor or asyncio.to_thread)
and await the result, replacing direct calls like
db_storage.load_user_data(user_id) with an awaited executor call, and do the
same for save_user_data/get_stale_users/delete_user_data throughout the
referenced functions to ensure non-blocking behavior.
- Around line 767-772: The safety_settings dict currently sets BLOCK_NONE for
all categories; update safety_settings to use a stricter default (e.g.,
genai.types.HarmBlockThreshold.BLOCK_MEDIUM_AND_ABOVE or at minimum
genai.types.HarmBlockThreshold.BLOCK_ONLY_HIGH) for sensitive categories such as
HARM_CATEGORY_HARASSMENT, HARM_CATEGORY_HATE_SPEECH,
HARM_CATEGORY_SEXUALLY_EXPLICIT and HARM_CATEGORY_DANGEROUS_CONTENT; keep the
dict name safety_settings and the genai.types.HarmCategory/HarmBlockThreshold
symbols so the rest of the code continues to reference the same keys and values.

---

Nitpick comments:
In `@src/db_storage.py`:
- Around line 23-33: Add a non-unique index on user_data.last_seen in the DB
schema setup to avoid full table scans during cleanup queries that filter by
last_seen; modify the table-creation block (user_data) to run a CREATE INDEX IF
NOT EXISTS idx_user_data_last_seen ON user_data(last_seen) and commit the change
so the cleanup/range-filter operations (the code that filters by last_seen)
become scalable.

In `@src/main.py`:
- Around line 902-905: The post_init currently launches cleanup_stale_users()
via asyncio.create_task without keeping the Task; create a module-level variable
(e.g., cleanup_task) and assign cleanup_task =
asyncio.create_task(cleanup_stale_users()) inside post_init (use global if
needed), then implement an async post_shutdown(app) that checks if cleanup_task
is not None and calls cleanup_task.cancel() (optionally await it or handle
CancelledError), and finally set application.post_shutdown = post_shutdown while
keeping application.post_init = post_init so the background task is cancelled on
shutdown.

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/db_storage.py`:
- Around line 86-88: The post_shutdown handler in main.py currently only cancels
the cleanup task and never closes the DB connection; update that handler to call
db_storage.close() (the close() method on the db_storage instance) so the
SQLite/DB connection is released on shutdown, ensuring you reference the same
db_storage object used elsewhere (e.g., the global or injected instance) and
wrap the call in a try/except to log any errors without preventing shutdown.

In `@src/main.py`:
- Around line 931-942: post_shutdown currently only cancels cleanup_task and
never closes the SQLite DB; update post_shutdown to also close the application's
DB connection: reference the same global DB handle (e.g., the module-level
sqlite connection variable used elsewhere) inside post_shutdown after
cancelling/awaiting cleanup_task, check it's not None, call its close() method
(handling any exceptions like OperationalError) and set the global to None so
the connection is fully released before shutdown; keep the existing
asyncio.CancelledError handling and ensure application.post_shutdown remains
assigned to post_shutdown.
- Around line 843-849: The fallback uses a hardcoded Ukrainian instruction when
calling model.generate_content; change it to build the instruction using the
existing language setting (instead of the literal "Відповідь українською
мовою...") so the fallback respects the user's language preference—modify the
call to model.generate_content (the block that produces simple_response and
checks simple_response.text) to interpolate/format the language variable with
the same intent ("respond in <language>: give general information about:
<prompt>") and keep passing safety_settings and prompt as before.
- Around line 801-835: The retry loop around model.generate_content can exit
without setting response if all rate-limit retries are exhausted, leading to an
UnboundLocalError; fix this by ensuring a clear failure path after the for
attempt in range(max_retries) loop: initialize response = None before the loop
(or check locals() after the loop), and if response is still None after retries,
send a final user-facing message via update.message.reply_text (same language
logic as the retry message) and then raise a specific exception (or return/exit)
to stop further processing instead of letting execution continue to where
response is used; update the code around model.generate_content, the except
block handling rate-limit, and the post-loop logic to implement this
deterministic failure behavior.

---

Outside diff comments:
In `@src/main.py`:
- Around line 1-10: The file-level formatting (top-level docstring and imports
in the module containing the triple-quoted string and imports like os, random,
json, asyncio, re, time, traceback, datetime) fails Black checks; run the
formatter with the exact CI command `black --skip-string-normalization
--line-length 120 src/` (or run it against this module) and commit the resulting
changes so the docstring and import block conform to project style.

---

Nitpick comments:
In `@src/db_storage.py`:
- Around line 77-84: Move the local import out of the get_stale_users method:
add "import time" to the module-level imports at the top of the file and remove
the "import time" statement inside the get_stale_users(self, ttl_seconds)
method; keep the function body otherwise unchanged (retain current_time =
time.time(), cursor usage, and return logic) to maintain behavior.
- Around line 36-49: The load_user_data method uses positional indexing on the
fetched row (row[1], row[2], etc.), which is fragile; change it to fetch columns
by name instead—either set the connection row factory to sqlite3.Row
(self.conn.row_factory = sqlite3.Row) and then access
row['conversation_context'], row['rate_limit_timestamps'], etc., or modify the
SQL to select explicit column names and map them to keys; update load_user_data
to use these named accesses (and keep the same JSON parsing and defaults) to
avoid breakage if the schema changes.
- Around line 12-18: The class currently creates and shares a single SQLite
connection (self.conn) in __init__, which causes "database is locked" under
concurrent asyncio.to_thread calls; change to a connection-per-operation pattern
by removing reliance on self.conn, add a helper method _get_connection(self)
that returns sqlite3.connect(self.db_path, check_same_thread=False) (or use
sqlite3.connect(..., timeout=...)) and update all DB methods (including
_create_tables, any read/write methods) to use a context manager like with
self._get_connection() as conn: instead of using self.conn; this ensures each
operation gets its own connection (or you can alternatively add a threading.Lock
around writes if preferred).

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/main.py`:
- Line 943: Remove the unnecessary global declaration to fix W0602: in the
function main(), delete the line declaring "global cleanup_task" (cleanup_task
is not assigned there); if you need to reference/modify cleanup_task in
post_shutdown(), keep or add "global cleanup_task" inside post_shutdown and
ensure you assign to cleanup_task there (e.g., set cleanup_task = ... before
using it) so the global is actually bound where it's modified.
- Around line 703-721: The db persistence call (db_storage.save_user_data
invoked via asyncio.to_thread) must be made best-effort so a failure does not
turn a successful LLM reply into a user-visible error: send the reply with
update.message.reply_text first, then perform persistence asynchronously and
swallow/log errors; e.g., move or duplicate the
asyncio.to_thread(db_storage.save_user_data, ...) call into a background
try/except (or an asyncio.create_task wrapper) that catches exceptions and logs
them (do not re-raise), so failures in save_user_data or the to_thread call do
not affect the previously sent reply.
- Around line 842-850: call_gemini_api is sending an error reply via
update.message.reply_text and then raising, which leads respond_with_llm_message
to send a second error; remove the user-facing reply from call_gemini_api and
instead raise a descriptive exception (or a custom GeminiRetryError) that
includes the localized fail_msg; then update respond_with_llm_message to catch
that exception and send a single reply using update.message.reply_text (using
the fail_msg from the exception) so only one error message is delivered. Ensure
references to call_gemini_api, respond_with_llm_message, and
update.message.reply_text are updated accordingly and handle both locations
noted (this block and the similar 723-748 path).
- Around line 891-915: The cleanup_stale_users background loop currently can die
on any exception (e.g., transient DB errors); wrap the loop body inside a
try/except that catches Exception around the await asyncio.sleep(...) + db calls
in cleanup_stale_users and ensure any exception is logged (including exception
details) but does not re-raise, so the loop continues; specifically protect
calls to db_storage.get_stale_users and db_storage.delete_user_data and the
in-memory deletions, log failures via the existing logger (include the
exception), and optionally add a short retry/backoff sleep on exception before
continuing the while True loop.

---

Duplicate comments:
In `@src/main.py`:
- Around line 795-799: The current harm configuration maps all
genai.types.HarmCategory entries to genai.types.HarmBlockThreshold.BLOCK_NONE;
update this mapping so public chatbot defaults use stricter thresholds per
Gemini guidance: set genai.types.HarmCategory.HARM_CATEGORY_HATE_SPEECH,
HARM_CATEGORY_SEXUALLY_EXPLICIT, and HARM_CATEGORY_DANGEROUS_CONTENT to
genai.types.HarmBlockThreshold.BLOCK_HIGH (or BLOCK_BLOCK if supported), and set
genai.types.HarmCategory.HARM_CATEGORY_HARASSMENT to a non‑NONE threshold such
as genai.types.HarmBlockThreshold.BLOCK_WARNING or
genai.types.HarmBlockThreshold.BLOCK_MODERATE; replace the BLOCK_NONE values in
the mapping in src/main.py accordingly and ensure the mapping variable is used
by the request setup so the new defaults take effect.